-
Notifications
You must be signed in to change notification settings - Fork 415
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test(job-scheduler): add tests with drain and promote for job scheduler #2944
base: master
Are you sure you want to change the base?
Conversation
tests/test_job_scheduler.ts
Outdated
|
||
await queue.promoteJobs(); | ||
|
||
expect( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about
const delayedCount = await queue.getDelayedCount();
const waitingCount = await queue.getWaitingCount();
expect(delayedCount).to.be.equal(1);
expect(waitingCount).to.be.equal(1);
by separating each expectation will be easily to identify where something is failing
tests/test_job_scheduler.ts
Outdated
expect( | ||
(await queue.getDelayedCount()) == 1 || | ||
(await queue.getWaitingCount()) == 1 || | ||
(await queue.getActiveCount()) == 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as worker is having a no-operational processor, it could process a waiting job really fast and no be in active state, I recommend removing worker instance and this check
|
||
await queue.upsertJobScheduler('scheduler-test', { | ||
every: 50000, | ||
immediately: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this option is not needed when using every option as it happens by default
tests/test_job_scheduler.ts
Outdated
(await queue.getWaitingCount()) == 1 || | ||
(await queue.getActiveCount()) == 1, | ||
).to.be.true; | ||
await queue.removeJobScheduler('scheduler-test'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may no be needed as afterEach hook is removing all keys for this queue
every: 50000, | ||
immediately: true, | ||
}); | ||
const worker = new Worker(queueName, async () => {}, { connection }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe initialize worker after drain operation as worker could take the first iteration as quick as possible
it('worker should start processing repeatable jobs after drain', async function () { | ||
await queue.upsertJobScheduler('scheduler-test', { | ||
every: 50000, | ||
immediately: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
immediately happens by default when using every option, no need to pass it
tests/test_job_scheduler.ts
Outdated
immediately: true, | ||
}); | ||
|
||
expect(worker.isRunning()).to.be.true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of this check, you are looking for checking that worker is able to process jobs from scheduler after drain operation, you can use a queueEvent instance to listen for completed state or from worker instance like in https://github.com/taskforcesh/bullmq/blob/master/tests/test_worker.ts#L1835-L1851
await queue.promoteJobs(); | ||
|
||
const waitingCount = await queue.getWaitingCount(); | ||
expect(waitingCount).to.be.equal(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to check that at least the next repeatable was scheduled you may need to check the delayed count as well, so after this line you can add:
const delayedCount = await queue.getDeleayedCount();
expect(delayedCount).to.be.equal(1);
}); | ||
}); | ||
|
||
const job = await queue.add('test', { foo: 'bar' }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this addition looks like it's not needed but you may need to check the completion of the scheduled job from scheduler
ref #247 #2405